-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Ensure that FileInfo return values as required by its phpdoc. #27389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@mrow4a, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975 and @PVince81 to be potential reviewers. |
d17393e to
959edc4
Compare
|
👍 |
| */ | ||
| public function getId() { | ||
| return $this->data['fileid']; | ||
| return isset($this->data['fileid']) ? intval($this->data['fileid']) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm didn't we say we also allow string file ids ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interface says clearly, INT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@labkode is that still a requirement ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PVince81 We are currently using int for file ids but we would like to use strings in the future.
The sync client already supports string file ids, if I remember correctly.
|
Regression on 32-bit systems: #28275 (comment) |
|
Hmm, if unit tests will pass after changing interface to STRING from INT, I am fine. And that phpdoc would need a proper explanation I guess. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This bug fix prevents the following scenario after calling
->getId()on FileInfo :Please merge it as soon as possible, I need it for other PRs